-
Notifications
You must be signed in to change notification settings - Fork 94
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Support public inputs in Ultra Honk #581
Conversation
808a4ce
to
fb6d7f8
Compare
34cbf10
to
b5b6dba
Compare
@@ -87,7 +89,7 @@ template <typename Flavor> class TranscriptTests : public testing::Test { | |||
round++; | |||
// TODO(Mara): Make testing more flavor agnostic so we can test this with all flavors | |||
if constexpr (IsGrumpkinFlavor<Flavor>) { | |||
manifest_expected.add_entry(round, "IPA:poly_degree", circuit_size); | |||
manifest_expected.add_entry(round, "IPA:poly_degree", size_uint64); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was a bug in the test that was revealed by the work in this PR but is otherwise unrelated.
@@ -9,6 +9,57 @@ struct SelectorProperties { | |||
bool requires_lagrange_base_polynomial = false; | |||
}; | |||
|
|||
/** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These two functions were formerly in the shared composer lib but they are used only for plonk so they've been moved to the plonk specific composer lib
@@ -1,179 +0,0 @@ | |||
#pragma once |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file was an exact copy of composer_lib and was not used at all so I've deleted it entirely.
85b2328
to
cc22a28
Compare
@@ -251,33 +249,6 @@ std::shared_ptr<typename Flavor::ProvingKey> UltraComposer_<Flavor>::compute_pro | |||
|
|||
// Polynomial memory is zeroed out when constructed with size hint, so we don't have to initialize trailing space | |||
|
|||
// // In the case of using UltraPlonkComposer for a circuit which does _not_ make use of any lookup tables, all |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the Plonk strategy for ensuring non-zero polys. We wont ever use this for Honk.
|
||
construct_selector_polynomials<Flavor>(circuit_constructor, proving_key.get()); | ||
|
||
// TODO(#217)(luke): Naively enforcing non-zero selectors for Honk will result in some relations not being |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the Plonk strategy for ensuring non-zero polys. We wont ever use this for Honk.
cc22a28
to
71901ac
Compare
@@ -36,6 +38,11 @@ template <UltraFlavor Flavor> class UltraComposer_ { | |||
std::vector<uint32_t> recursive_proof_public_input_indices; | |||
bool contains_recursive_proof = false; | |||
bool computed_witness = false; | |||
size_t total_num_gates = 0; // num_gates + num_pub_inputs + tables + zero_row_offset (used to compute dyadic size) | |||
size_t dyadic_circuit_size = 0; // final dyadic circuit size |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd recommend adding (next power of two) in the comments. As a variable name dyadic is ok, but for me it wasn't clear from the start if this is supposed to be the logarithm or the power of 2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good call - done
@@ -66,6 +66,9 @@ class Standard { | |||
using RelationUnivariates = decltype(create_relation_univariates_container<FF, Relations>()); | |||
using RelationValues = decltype(create_relation_values_container<FF, Relations>()); | |||
|
|||
// Whether or not the first row of the execution trace is reserved for 0s to enable shifts | |||
static constexpr bool has_zero_row = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we need it for all honk flavors?
// define utilities to extend univarates from RELATION_LENGTH to MAX_RELATION_LENGTH for each Relation | ||
// using BarycentricUtils = decltype(create_barycentric_utils<FF, Relations, MAX_RELATION_LENGTH>()); | ||
// Whether or not the first row of the execution trace is reserved for 0s to enable shifts | ||
static constexpr bool has_zero_row = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we need it for all honk flavors?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need for the zero row for Standard flavors since in this case the only shifted poly is z_perm which is constructed explicitly with a constant coeff equal to 0. The problem arises for UltraHonk since we need to shift the wire polys themselves
Description
The primary goal of this PR is to support public inputs in Ultra Honk. This was previously not supported because adding a non-zero public input to an UH circuit would result in wires with a non-zero first entry, making them not shiftable. The solution is to add a "zero row" at the start of the execution trace for all UH circuits. The ordering of the execution trace is thus: zero row, public inputs, conventional gates.
A portion of the work in this PR is simply cleanup that was necessary to limit the number of places in the code where we need to specify the "zero row offset". For example, this offset has to be taken into account in the computation of the dyadic circuit size, which was previously computed independently in 3 separate locations within the Honk composers.
Note about implementation: In general there are many "zero rows" in every circuit, i.e. the padding between the "filled" rows of the execution trace and the dyadic (power of 2) circuit size. For this reason, 0 is added as a variable in the circuit builders by default. To add a zero row at the start of the execution trace, I simply add the first index of each wire to the copy cycle associated with the variable 0. Whether or not this zero row is utilized is specified in the Flavor, since, for example, it is not needed for plonk variants or even Standard Honk.
Checklist:
@brief
describing the intended functionality.include
directives have been added.